linker: detect duplicate exported definitions without imports#6750
linker: detect duplicate exported definitions without imports#6750Lurie97 wants to merge 1 commit into
Conversation
|
|
The duplicate-export check in GetImportExportPairs() only triggers when iterating over imports. If no module imports a symbol, the linker never looks up the export table for that name, so multiple modules can each export the same function and spvtools::Link() silently accepts them instead of reporting a One Definition Rule violation. Add a standalone pass over the exports map before the import-matching loop to reject links where the same symbol name is exported by more than one module. This correctly handles LinkOnceODR symbols because they are already deduplicated into a single export entry by the preceding linkonce processing. Fixes the piglit clLinkProgram test where two modules each define get_number() with Export linkage but neither imports it. Signed-off-by: jiajia Qian <jiajia.qian@nxp.com>
s-perron
left a comment
s-perron
left a comment
There was a problem hiding this comment.
Please add a test as well.
| << possible_export.second.name << "\"."; | ||
| } | ||
|
|
||
| // Detect duplicate exported definitions (One Definition Rule violation). |
There was a problem hiding this comment.
Can you point to the "One definition rule"? When talking about linking, the ODR rule is a C++ rule that says a function can have multiple declarations, but they must all be the same. When the linker sees two definitions, it is allowed to pick one at random, and it will not change the semantics of the program.
Also, a C++-like ODR feature in part of SPIR-V.
You might want to change the comment to say that you cannot have multiple strong definitions.
| else if (possible_exports.size() > 1u) | ||
| return DiagnosticStream(position, consumer, "", SPV_ERROR_INVALID_BINARY) | ||
| << "Too many external references, " << possible_exports.size() | ||
| << ", were found for \"" << import.name << "\"."; |
There was a problem hiding this comment.
Do these check become redundant? I'm guessing this code could be removed.
3 participants
The duplicate-export check in GetImportExportPairs() only triggers when iterating over imports. If no module imports a symbol, the linker never looks up the export table for that name, so multiple modules can each export the same function and spvtools::Link() silently accepts them instead of reporting a One Definition Rule violation.
Add a standalone pass over the exports map before the import-matching loop to reject links where the same symbol name is exported by more than one module. This correctly handles LinkOnceODR symbols because they are already deduplicated into a single export entry by the preceding linkonce processing.
Fixes the piglit clLinkProgram test where two modules each define get_number() with Export linkage but neither imports it.